Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle when auto updater receives a non-200 from GitHub API #351

Merged
merged 1 commit into from
Dec 22, 2023
Merged

fix: handle when auto updater receives a non-200 from GitHub API #351

merged 1 commit into from
Dec 22, 2023

Conversation

jamesvl
Copy link
Contributor

@jamesvl jamesvl commented Dec 22, 2023

This is a minimal fix to the crash reported in #350 - e.g. Github API returns a 403 or other non-200 error. (Github's API docs do not make it easy to find the response schema for errors, only successes, unfortunately.)

Fixes #350

@@ -65,6 +65,12 @@ defmodule NextLS.Updater do
end
end

{:ok, %{status: 403, body: %{"message" => msg}} = _body} ->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding these two specific checks, let's make the existing check less specific (don't pattern match on the error atom).

Keep the status 200 in the first check tho

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, no problem. By "error atom" do you mean the status field, or the "message" field? (I wasn't sure if Github returns a message for non-403 responses.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I mean the atom on line 74

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@mhanberg mhanberg changed the title Handle case when Req gets non-200 Github API response fix: handle case when Req gets non-200 Github API response Dec 22, 2023
@mhanberg mhanberg changed the title fix: handle case when Req gets non-200 Github API response fix: handle when auto updater receives a non-200 from GitHub API Dec 22, 2023
@mhanberg mhanberg enabled auto-merge (squash) December 22, 2023 15:34
@mhanberg mhanberg merged commit 3564971 into elixir-tools:main Dec 22, 2023
6 of 8 checks passed
@mhanberg
Copy link
Collaborator

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NextLS.DynamicSupervisor crashes on 403 Github API response
2 participants